Make country package purely deterministic - read stochastic variables from dataset #6635
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
- Rename /review-pr to /fix-pr (applies fixes automatically) - Create new /review-pr (posts GitHub review without changes) - New /review-pr uses gh api to post inline comments with code suggestions - Includes emoji categories: 💡 hard-coded values, 🧪 tests, 📚 docs, ⚡ performance 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <[email protected]>
…lue rules - Add vectorization rules to policy-domain-validator: if statements are fine for scalar values (period, parameters), problematic for arrays - Clarify acceptable hard-coded values: bootstrapping values, mathematical baselines, structural constants, framework constants - Update implementation-validator with same guidance - Add duplicate review detection to review-pr command to avoid multiple review comments - Distinguish between MONTHS_IN_YEAR constant (preferred) vs literal 12 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <[email protected]>
… from dataset This change removes all random number generation from policyengine-us. All stochastic take-up variables are now generated in policyengine-us-data and read from the dataset. The country package is now a purely deterministic rules engine. ## Key Changes ### Removed - All take-up seed variables (snap_take_up_seed, aca_take_up_seed, medicaid_take_up_seed) - All take-up rate parameters (moved to policyengine-us-data) ### Simplified All takes_up_* variables now use dataset values with deterministic fallbacks: - takes_up_snap_if_eligible (default: True) - takes_up_aca_if_eligible (default: True) - takes_up_medicaid_if_eligible (default: True) ## Trade-offs **IMPORTANT**: Take-up rates can no longer be adjusted dynamically via policy reforms or in the web app. They are fixed in the microdata. This is an acceptable trade-off for the cleaner architecture of keeping the country package purely deterministic. To adjust take-up rates for analysis, the microdata must be regenerated with updated parameter values in policyengine-us-data. Related: policyengine-us-data PR (must be merged FIRST) 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <[email protected]>
Collaborator
|
I think the Mass branch got merged into this PR @MaxGhenis |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Summary
This PR removes all random number generation from policyengine-us. All stochastic take-up variables are now generated in policyengine-us-data and read from the dataset. The country package is now a purely deterministic rules engine.
Changes
Removed
Simplified
All takes_up_* variables now use dataset values with deterministic fallbacks:
These variables have no formula - when present in the dataset, OpenFisca uses the dataset value. For policy calculator (non-microsimulation), they default to True (full take-up assumption).
Trade-offs
IMPORTANT: Take-up rates can no longer be adjusted dynamically via policy reforms or in the web app. They are fixed in the microdata. This is an acceptable trade-off for the cleaner architecture of keeping the country package purely deterministic.
To adjust take-up rates for analysis, the microdata must be regenerated with updated parameter values in policyengine-us-data.
Test Plan
Related PRs
🤖 Generated with Claude Code
Co-Authored-By: Claude [email protected]